Add `InheritEnvironmentVariables` to `StdioClientTransportOptions` by halter73 · Pull Request #1563 · modelcontextprotocol/csharp-sdk

halter73 · 2026-05-06T05:34:51Z

Summary

Introduces a new bool InheritEnvironmentVariables { get; set; } = true property on StdioClientTransportOptions that controls whether the child server process inherits the current process's environment variables.

Motivation

Previously, EnvironmentVariables only augmented/overwrote inherited vars — there was no way to start the server with a clean environment. Retconning the dictionary to mean "only these vars" would be breaking.

Behavior

`InheritEnvironmentVariables`	`EnvironmentVariables`	Result
`true` (default)	`null`	Child inherits all parent env vars (unchanged)
`true` (default)	`{ "X": "y" }`	Child inherits all + `X=y` on top
`false`	`null`	Child starts with empty environment
`false`	`{ "PATH": "...", "API_KEY": "..." }`	Child only sees the explicitly provided vars

When false, startInfo.Environment.Clear() is called before any EnvironmentVariables entries are applied.

Security / Compatibility note

The docs and XML comments explain both sides:

Security risk (inheriting): Credentials like AWS_SECRET_ACCESS_KEY, GITHUB_TOKEN, OPENAI_API_KEY automatically flow into the child process.
Compatibility risk (disabling): PATH, HOME, DOTNET_ROOT, LD_LIBRARY_PATH, JAVA_HOME, proxy vars etc. are required by many tools — forgetting them causes the process to fail to start.

Changes

StdioClientTransportOptions.cs — new InheritEnvironmentVariables property with comprehensive XML docs
StdioClientTransport.cs — calls startInfo.Environment.Clear() when !InheritEnvironmentVariables
StdioClientTransportTests.cs — 3 new tests verifying inherit, block, and explicit-only scenarios
transports.md — new property in the options table + "Environment variable inheritance" section with code sample and [!WARNING] callout

Introduces a new bool property InheritEnvironmentVariables (default: true) on StdioClientTransportOptions that controls whether the child server process inherits the current process's environment variables. When false, startInfo.Environment.Clear() is called before any EnvironmentVariables entries are applied, giving the child process a completely clean environment. This allows callers to prevent unintentional leakage of credentials, tokens, and proxy settings into third-party or untrusted MCP server processes. The new property is backward-compatible: the default (true) preserves the existing behavior of inheriting all parent env vars and then layering EnvironmentVariables on top. Three tests are added to StdioClientTransportTests: - InheritEnvironmentVariables_DefaultTrue_ChildSeesParentEnvVars - InheritEnvironmentVariables_False_ChildDoesNotSeeParentEnvVars - InheritEnvironmentVariables_False_WithExplicitVars_ChildSeesOnlyExplicitVars Documentation in transports.md is updated with: - New property in the stdio options table - A dedicated 'Environment variable inheritance' section with a code sample and a WARNING callout explaining both the security risk of inheriting (credential leakage) and the compatibility risk of disabling (PATH, HOME, DOTNET_ROOT etc. required by many tools). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Instead of calling Environment.SetEnvironmentVariable (which is process-global and can cause race conditions with parallel tests), rely on PATH which is always present in a real process environment as the 'known parent variable'. For the no-inherit tests, use absolute shell paths (/bin/sh on Unix, full cmd.exe path on Windows from SystemRoot) so the child can launch even with a completely empty environment — no PATH needed. Test 1: default inheritance → child sees PATH (no parent env mutation) Test 2: InheritEnvironmentVariables=false → child does NOT see PATH Test 3: InheritEnvironmentVariables=false + explicit var → PATH absent, explicit MCP_STDIO_TEST_EXPLICIT_VAR is present Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…te paths Use PATH (passed explicitly when InheritEnvironmentVariables=false) to find cmd/sh rather than hardcoded absolute paths. Verify inheritance by checking variables that are always present in a real process but deliberately omitted from the explicit EnvironmentVariables dict: HOME (Unix) and USERNAME (Windows). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Both QuickstartClient and ChatWithTools now disable environment variable inheritance and forward only the variables their server processes require: PATH, HOME/USERPROFILE, APPDATA, LOCALAPPDATA, TEMP/TMPDIR for Node/npm; plus DOTNET_ROOT and NUGET_PACKAGES for the dotnet case. This prevents credentials, tokens, and other sensitive parent-process variables from leaking into third-party MCP server processes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…se test Both DefaultTrue and False tests now check HOME (Unix) / USERNAME (Windows) so they form a symmetric pair asserting opposite outcomes on the same signal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Returns a curated allowlist of env vars (PATH, HOME, system dirs, etc.) safe to forward to child processes, aligned with the TypeScript and Python MCP SDK defaults. Replaces the duplicated MinimalEnvironment() helpers in the samples and updates transports.md to demonstrate the new API. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…ocs and XML Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

OrdinalIgnoreCase on Windows to match ProcessStartInfo.Environment behavior, Ordinal on Unix where env var names are case-sensitive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

tarekgh · 2026-05-06T15:49:24Z

Do we have an option to exclude specific environment variables? For example, could we inherit all variables via InheritEnvironmentVariables while explicitly excluding a subset? Alternatively, is this something that must be handled manually? I’m asking because this capability could help mitigate the security risk mentioned here.

tarekgh · 2026-05-06T17:23:40Z

 | `Name` | Optional transport identifier for logging |

+#### Environment variable inheritance
+


Do we need to mention anything regarding Windows shell wrapping?

tarekgh · 2026-05-06T20:04:20Z

    messages.AddMessages(updates);
-}
+}
+


nit: extra empty lines

tarekgh · 2026-05-06T20:04:49Z

+// Returns the safe default environment variables plus extras needed by 'dotnet run'.
+// Omitting variables the server doesn't need prevents unintentional leakage of
+// credentials or other sensitive values present in the parent process.
+static Dictionary<string, string?> MinimalDotNetEnvironment()


nit: call it GetMinimalDotNetEnvironment?

tarekgh · 2026-05-06T20:09:34Z

@@ -150,6 +150,111 @@ public async Task EscapesCliArgumentsCorrectly(string? cliArgumentValue)
        Assert.Equal(cliArgumentValue ?? "", content.Text);
    }



As GetDefaultEnvironmentVariables() is public method, should we have dedicated test for it? it can test:

Shell function filtering (value.StartsWith("()"))

Platform comparer (OrdinalIgnoreCase on Windows vs Ordinal on Unix)

Returns a fresh dictionary each call

Only includes allowlisted vars that exist, excludes others

tarekgh

Added minor comments and question. LGTM, otherwise.

ericstj · 2026-05-07T16:17:22Z

+> [!WARNING]
+> **Security risk (inheriting):** Variables such as `AWS_SECRET_ACCESS_KEY`, `GITHUB_TOKEN`, `OPENAI_API_KEY`, and similar credentials present in the parent process automatically flow into the child process unless inheritance is disabled. This can unintentionally expose sensitive values to third-party or untrusted MCP servers.
+>
+> **Compatibility risk (not inheriting):** Disabling inheritance can cause the child process to fail to start or behave incorrectly if it relies on variables provided by the OS or shell. `GetDefaultEnvironmentVariables()` covers the most common requirements — `PATH`, `HOME`, and standard system directories — so for most servers it is a safe starting point. For servers that need additional variables not in the default set (such as `DOTNET_ROOT`, `LD_LIBRARY_PATH`, `JAVA_HOME`, or proxy settings like `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY`), add them on top as shown in the example above.


I wonder if we want to roll out a breaking change around this in a future major version. I could imagine an analyzer that tells folks to set this to false. Let that percolate a couple releases, then change the default and update the analyzer to only flag when folks set it to true.

That might make sense. We are thinking about doing something similar for changing the stateless defaults. See #1471. I need to update that, so I could include something for this there as well.

ericstj · 2026-05-07T16:21:59Z

+> **Security risk (inheriting):** Variables such as `AWS_SECRET_ACCESS_KEY`, `GITHUB_TOKEN`, `OPENAI_API_KEY`, and similar credentials present in the parent process automatically flow into the child process unless inheritance is disabled. This can unintentionally expose sensitive values to third-party or untrusted MCP servers.
+>
+> **Compatibility risk (not inheriting):** Disabling inheritance can cause the child process to fail to start or behave incorrectly if it relies on variables provided by the OS or shell. `GetDefaultEnvironmentVariables()` covers the most common requirements — `PATH`, `HOME`, and standard system directories — so for most servers it is a safe starting point. For servers that need additional variables not in the default set (such as `DOTNET_ROOT`, `LD_LIBRARY_PATH`, `JAVA_HOME`, or proxy settings like `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY`), add them on top as shown in the example above.
+


suggestion: Might be worth while showing the pattern to selectively inherit. Maybe for a set of variables by name, copy those to env

- Add 6 unit tests for GetDefaultEnvironmentVariables() covering: fresh dictionary per call, correct platform comparer, only allowlisted keys, no shell function values, PATH present, non-allowlisted keys absent - Add selective inheritance code example to transports.md - Mention PATHEXT in docs explanation (Windows .cmd/.bat support) - Rename MinimalDotNetEnvironment -> GetMinimalDotNetEnvironment - Remove trailing blank line in ChatWithTools/Program.cs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Deconstruct() on KeyValuePair<T,U> is not available on .NET Framework 4.7.2. Use kvp.Key / kvp.Value instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

halter73 and others added 2 commits May 5, 2026 22:34

halter73 requested review from ericstj, jeffhandley and tarekgh May 6, 2026 05:57

halter73 and others added 6 commits May 5, 2026 23:08

Point compatibility warnings at GetDefaultEnvironmentVariables() in d…

d6505bb

…ocs and XML Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Use platform-aware comparer in GetDefaultEnvironmentVariables()

0ea6451

OrdinalIgnoreCase on Windows to match ProcessStartInfo.Environment behavior, Ordinal on Unix where env var names are case-sensitive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

tarekgh reviewed May 6, 2026

View reviewed changes

tarekgh previously approved these changes May 6, 2026

View reviewed changes

ericstj reviewed May 7, 2026

View reviewed changes

ericstj previously approved these changes May 7, 2026

View reviewed changes

halter73 dismissed stale reviews from ericstj and tarekgh via 4614095 May 7, 2026 20:01

halter73 and others added 2 commits May 7, 2026 13:29

Fix net472 build: avoid KeyValuePair deconstruction in test

3191335

Deconstruct() on KeyValuePair<T,U> is not available on .NET Framework 4.7.2. Use kvp.Key / kvp.Value instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Trigger CI re-run (auth/metadata-var1 flaky timeout)

f696235

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `InheritEnvironmentVariables` to `StdioClientTransportOptions`#1563

Add `InheritEnvironmentVariables` to `StdioClientTransportOptions`#1563
halter73 wants to merge 11 commits intomainfrom
halter73/stdio-env-inherit-option

halter73 commented May 6, 2026

Uh oh!

tarekgh commented May 6, 2026

Uh oh!

tarekgh May 6, 2026

Uh oh!

tarekgh May 6, 2026

Uh oh!

tarekgh May 6, 2026

Uh oh!

tarekgh May 6, 2026

Uh oh!

tarekgh left a comment

Uh oh!

ericstj May 7, 2026

Uh oh!

halter73 May 7, 2026

Uh oh!

ericstj May 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		\| `Name` \| Optional transport identifier for logging \|

		#### Environment variable inheritance

		@@ -150,6 +150,111 @@ public async Task EscapesCliArgumentsCorrectly(string? cliArgumentValue)
		Assert.Equal(cliArgumentValue ?? "", content.Text);
		}

Conversation

halter73 commented May 6, 2026

Summary

Motivation

Behavior

Security / Compatibility note

Changes

Uh oh!

tarekgh commented May 6, 2026

Uh oh!

tarekgh May 6, 2026

Choose a reason for hiding this comment

Uh oh!

tarekgh May 6, 2026

Choose a reason for hiding this comment

Uh oh!

tarekgh May 6, 2026

Choose a reason for hiding this comment

Uh oh!

tarekgh May 6, 2026

Choose a reason for hiding this comment

Uh oh!

tarekgh left a comment

Choose a reason for hiding this comment

Uh oh!

ericstj May 7, 2026

Choose a reason for hiding this comment

Uh oh!

halter73 May 7, 2026

Choose a reason for hiding this comment

Uh oh!

ericstj May 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants